New sniff to avoid deprecated "no"/"yes" autoload parameters when cal…#2893
Conversation
…ling update_option
📝 WalkthroughWalkthroughThe PR standardizes the autoload parameter in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/UpdateOptionAutoloadBooleanSniff.php`:
- Around line 142-146: The level tracking in UpdateOptionAutoloadBooleanSniff is
incrementing for T_OPEN_SHORT_ARRAY but not decrementing for its counterpart;
update the conditional that decrements $level (currently checking
T_CLOSE_PARENTHESIS and T_CLOSE_SQUARE_BRACKET) to also check for
T_CLOSE_SHORT_ARRAY so short-array brackets "[]" properly balance with the
T_OPEN_SHORT_ARRAY token during argument parsing.
- Around line 150-152: The condition using "false === $argStart" in
UpdateOptionAutoloadBooleanSniff is wrong because $argStart is initialized to
null, so update the check to "null === $argStart" (or "! isset($argStart)" /
"$argStart === null") so the condition can ever be true, or simply remove the
entire "false === $argStart" branch if the later null-check already covers this
case; target the comparison using the $argStart variable and the existing "false
=== $argStart" expression for the change.
| if ( $code === T_OPEN_PARENTHESIS || $code === T_OPEN_SHORT_ARRAY || $code === T_OPEN_SQUARE_BRACKET ) { | ||
| $level++; | ||
| } elseif ( $code === T_CLOSE_PARENTHESIS || $code === T_CLOSE_SQUARE_BRACKET ) { | ||
| $level--; | ||
| } |
There was a problem hiding this comment.
Missing T_CLOSE_SHORT_ARRAY in level decrement.
The level tracking correctly increments for T_OPEN_SHORT_ARRAY but the decrement only handles T_CLOSE_PARENTHESIS and T_CLOSE_SQUARE_BRACKET. This could cause incorrect argument parsing for code containing short array syntax [...].
🐛 Proposed fix
if ( $code === T_OPEN_PARENTHESIS || $code === T_OPEN_SHORT_ARRAY || $code === T_OPEN_SQUARE_BRACKET ) {
$level++;
- } elseif ( $code === T_CLOSE_PARENTHESIS || $code === T_CLOSE_SQUARE_BRACKET ) {
+ } elseif ( $code === T_CLOSE_PARENTHESIS || $code === T_CLOSE_SQUARE_BRACKET || $code === T_CLOSE_SHORT_ARRAY ) {
$level--;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( $code === T_OPEN_PARENTHESIS || $code === T_OPEN_SHORT_ARRAY || $code === T_OPEN_SQUARE_BRACKET ) { | |
| $level++; | |
| } elseif ( $code === T_CLOSE_PARENTHESIS || $code === T_CLOSE_SQUARE_BRACKET ) { | |
| $level--; | |
| } | |
| if ( $code === T_OPEN_PARENTHESIS || $code === T_OPEN_SHORT_ARRAY || $code === T_OPEN_SQUARE_BRACKET ) { | |
| $level++; | |
| } elseif ( $code === T_CLOSE_PARENTHESIS || $code === T_CLOSE_SQUARE_BRACKET || $code === T_CLOSE_SHORT_ARRAY ) { | |
| $level--; | |
| } |
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/UpdateOptionAutoloadBooleanSniff.php`
around lines 142 - 146, The level tracking in UpdateOptionAutoloadBooleanSniff
is incrementing for T_OPEN_SHORT_ARRAY but not decrementing for its counterpart;
update the conditional that decrements $level (currently checking
T_CLOSE_PARENTHESIS and T_CLOSE_SQUARE_BRACKET) to also check for
T_CLOSE_SHORT_ARRAY so short-array brackets "[]" properly balance with the
T_OPEN_SHORT_ARRAY token during argument parsing.
| if ( false === $argStart ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Type mismatch in comparison - $argStart initialized as null but compared to false.
$argStart is initialized as null on line 135, but the condition on line 150 checks false === $argStart. In PHP, false === null evaluates to false, so this condition will never be true. This appears to be dead code or a bug.
🐛 Proposed fix
- if ( false === $argStart ) {
+ if ( null === $argStart && $tokens[ $i ]['code'] === T_WHITESPACE ) {
continue;
}Or simply remove this block if the intent is already handled by line 169's condition.
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/UpdateOptionAutoloadBooleanSniff.php`
around lines 150 - 152, The condition using "false === $argStart" in
UpdateOptionAutoloadBooleanSniff is wrong because $argStart is initialized to
null, so update the check to "null === $argStart" (or "! isset($argStart)" /
"$argStart === null") so the condition can ever be true, or simply remove the
entire "false === $argStart" branch if the later null-check already covers this
case; target the comparison using the $argStart variable and the existing "false
=== $argStart" expression for the change.
…ling update_option
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.